-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
chore(logs): update SDK onboarding pages #104234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #104234 +/- ##
===========================================
- Coverage 80.60% 80.60% -0.01%
===========================================
Files 9333 9333
Lines 398600 398597 -3
Branches 25497 25496 -1
===========================================
- Hits 321275 321272 -3
Misses 76875 76875
Partials 450 450 |
| import {StepType} from 'sentry/components/onboarding/gettingStartedDoc/types'; | ||
| import {t, tct} from 'sentry/locale'; | ||
|
|
||
| type Params = DocsParams; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can use the DocsParams directly instead of using type Params
| 'cocoa-objc', | ||
| 'cocoa-swift', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that we have several references to these platforms in the codebase, but we don’t present them as options when users create a project. My understanding is that users can also create projects through the public API - Is that why we need to include these platforms here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah seems like it, they were also added to some other categories for backwards compatability #84768
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
- fix link - add minidump storage setting
8411a74 to
f3a368a
Compare
| { | ||
| type: StepType.VERIFY, | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: t( | ||
| 'Once structured logging is enabled, you can send logs using the AddLog method on the Sentry subsystem.' | ||
| ), | ||
| }, | ||
| { | ||
| type: 'code', | ||
| language: 'cpp', | ||
| code: `USentrySubsystem* SentrySubsystem = GEngine->GetEngineSubsystem<USentrySubsystem>(); | ||
| // Send logs at different severity levels | ||
| SentrySubsystem->AddLog(TEXT("A simple log message"), ESentryLevel::Info, TEXT("GameFlow")); | ||
| SentrySubsystem->AddLog(TEXT("Failed to save game data"), ESentryLevel::Error, TEXT("SaveSystem"));`, | ||
| }, | ||
| { | ||
| type: 'text', | ||
| text: tct( | ||
| 'You can also automatically capture Unreal Engine [code:UE_LOG] calls. For more information, see the [link:Automatic UE_LOG Integration documentation].', | ||
| { | ||
| code: <code />, | ||
| link: ( | ||
| <ExternalLink href="https://docs.sentry.io/platforms/unreal/logs/#automatic-ue_log-integration" /> | ||
| ), | ||
| } | ||
| ), | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we currently repeat this exact text in onboarding.tsx. I’m wondering if it would be better to move it to unreal/utils.tsx (needs to be created) and reuse it. That would make maintenance easier.
| text: tct( | ||
| 'To enable structured logging, check the [code:Enable Logs] option in the Sentry configuration window.', | ||
| {code: <code />} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this? I notice that the code snippet already gets updated when "logs" is checked
| { | ||
| type: 'text', | ||
| text: t( | ||
| 'Once structured logging is enabled, you can send logs using the AddLog method on the Sentry subsystem.' | ||
| ), | ||
| }, | ||
| { | ||
| type: 'code', | ||
| language: 'cpp', | ||
| code: `USentrySubsystem* SentrySubsystem = GEngine->GetEngineSubsystem<USentrySubsystem>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than adding a separate section just for logs, shouldn’t we update the verify code snippet to include the log’s code? javascript-react is doing that for example
| sentry_options_set_dsn(options, "${params.dsn.public}"); | ||
| // This is also the default-path. For further information and recommendations: | ||
| // https://docs.sentry.io/platforms/native/configuration/options/#database-path | ||
| // https://docs.sentry.io/platforms/native/configuration/options/#database_path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch here!
| { | ||
| title: t('Further Settings'), | ||
| content: [ | ||
| { | ||
| type: 'custom', | ||
| content: ( | ||
| <StoreCrashReportsConfig | ||
| organization={params.organization} | ||
| projectSlug={params.project.slug} | ||
| /> | ||
| ), | ||
| }, | ||
| ], | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this be in a separate PR or its related?
| { | ||
| link: ( | ||
| <ExternalLink href="https://github.com/getsentry/sentry-native/releases/tag/0.11.1" /> | ||
| ), | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
when mentioning versions, why do provide a link only here? asking because I did not see that for dotnet too.
| { | ||
| type: 'text', | ||
| text: t( | ||
| 'Once the feature is enabled on the SDK and the SDK is initialized, you can send logs using the SentrySdk.Logger APIs.' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'Once the feature is enabled on the SDK and the SDK is initialized, you can send logs using the SentrySdk.Logger APIs.' | |
| 'Once the feature is enabled and the SDK is initialized, you can send logs using the SentrySdk.Logger APIs.' |
| ProductSolution.PROFILING, | ||
| ProductSolution.LOGS, | ||
| ], | ||
| 'dotnet-aspnet': [ProductSolution.PERFORMANCE_MONITORING], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we going to add LOGS here and other platforms afterwards? Just noticed that it's "new" in our docs https://docs.sentry.io/platforms/dotnet/guides/aspnet/logs/
Continuing from #102398, to be merged after that one was merged.
We add the
logsonboarding for each of the platforms that now support logs (unity, unreal, dotnet, native, php-symfony)ToDo
logscheckbox to project onboarding page for each of these SDKs (see original comment)